I believe the return value is void?

This function also needs to be added into the types folder (and later we need just generate-types)

never is correct since the function does not return.

Would you mind adding a comment above this line?

Nit:

Don't use jsg::Lock::from(). Have the first argument to the method be jsg::Lock& js -- it'll be filled in by the JSG layer.

Copy seems unnecessary: Since it's not a dynamic worker, the def should be long-lived.

This is copying the whole source code of the worker! Just to support this feature. It's probably unnecessary (see previous comment).

[HIGH] Previously flagged issue not addressed: kentonv commented that jsg::Lock::from() should not be used — instead, the method should take jsg::Lock& js as its first parameter, which JSG fills in automatically.

The current code uses jsg::Lock::from(v8::Isolate::GetCurrent()) which is the discouraged pattern. The fix is straightforward:

should become:

  js.terminateExecutionNow();

and the method signature (in the .h) should change to:

void abortIsolate(jsg::Lock& js, jsg::Optional<kj::String> reason);

JSG will automatically provide the jsg::Lock& argument — it won't be visible to JavaScript callers.

Sigh.. very helpful bonk

[MEDIUM] Stale comment. This says "The current request continues running on the old isolate" but the actual implementation (per jasnell/kentonv's feedback) aborts ALL in-flight requests immediately. The comment should read something like:

Throws away the current JS isolate and recreates the worker from scratch. All in-flight requests on the isolate are immediately terminated. Subsequent requests will use a fresh isolate with re-executed top-level module code and fresh global state.

The same stale wording appears in io-channels.h:279-281.

[HIGH] OwnedWorkerSource drops several fields from ModulesSource:

1. capnpSchemas (capnp::List<capnp::schema::Node>::Reader) — not copied. This is a non-owning capnp reader that points into the original config message. While Cap'n Proto modules are marked "TODO(someday)" for workerd, if someone does use a CapnpModule, this will be a dangling reader. At minimum, the field should be initialized to an empty list or an orphan copy.

2. pythonMemorySnapshot (kj::Maybe<capnp::AnyStruct::Reader>) — not copied. If a Python worker uses abortIsolate(), the recreated worker will be missing its memory snapshot.

3. CommonJsModule::namedExports (kj::Maybe<kj::Array<kj::StringPtr>>) — the CJS clone at line 4541 only copies .body but drops namedExports. These are non-owning StringPtrs into the body, so they'd need to be reconstructed after cloning the body.

4. EsModule::ownBody (kj::Maybe<::rust::String>) — line 4535 copies from es.body (the ArrayPtr<const char>) which works if ownBody is kj::none (body points into capnp message). But if ownBody is set (meaning body was transpiled from Rust and points into ownBody), the copy via kj::str(es.body) will correctly capture the content. However, the original ownBody is silently dropped, which is fine since the clone owns the string. Just noting for completeness — this case is actually handled correctly.

Items 1-3 are bugs.

[HIGH] Previously flagged issue not addressed: kentonv commented that the deep copy of the worker source code here is unnecessary. For a static (non-dynamic) worker, the capnp config message that contains the source lives for the entire lifetime of the Server. The `WorkerSource` borrows from it, and that borrow remains valid.

Rather than the entire `OwnedWorkerSource` machinery (~100 lines), you could just store a `const WorkerSource&` reference to `def.source` (or a copy of the lightweight `WorkerSource` struct itself, which only contains non-owning pointers). The source data it points to lives in the config message, which outlives the `WorkerService`.

If the concern is that `def` gets moved into `linkCallback`, then extract the `WorkerSource` reference before that move (as is already done for other fields). The underlying data remains valid regardless of where the `WorkerSource` struct lives.

This would eliminate the `OwnedWorkerSource` class entirely and address kentonv's concern about unnecessary copies of the entire source code.

[MEDIUM] The worker factory skips several steps that makeWorkerImpl() performs on the original worker:

1. preloadPython() (line 4735) — not called in the factory. If this is a Python worker, the recreated worker won't have preloaded Python artifacts.

2. validateHandlers() (line 4774 in the original path) — not called in the factory. Handler validation errors will be silently skipped on recreation.

3. Actor namespace initialization — the factory calls createWorker() but doesn't call initActorNamespaces() or set up actor channels. If the worker has Durable Objects, the recreated worker may not have them properly configured. (Though the existing WorkerService already has the actor namespaces, so this might be OK if they don't need re-initialization — worth verifying.)

4. Inspector registration — createWorker() does register with the inspector (good), but if the old isolate's inspector session is still active, there could be confusion. Worth considering.

Items 1-2 are potential bugs for Python workers and workers with unusual handler configurations.

[MEDIUM] This clones the env globals eagerly for every worker, even though it's only used when abortIsolate() is called (which most workers never do). This is the `envGlobalsForFactory` field.

Given that for static workers the `def` is moved into `linkCallback` anyway, and the underlying config data is long-lived, consider whether the globals could be cloned lazily (only on first `abortIsolate()` call) or borrowed from the config message. The `Global::clone()` does deep copies of strings and byte arrays for each binding, which is unnecessary work for the common case.

[MEDIUM] Race window between abortIsolate() and startRequest(). Consider: request A calls abortIsolate(), which sets isolateAbortRequested = true and rejects the abort-all promise. Request B arrives and calls startRequest(), which sees the flag, calls workerFactory() to create a new worker, and resets the flag. Meanwhile, the abort-all rejection is still propagating to other in-flight requests on the old isolate.

The factory call (workerFactory()) is synchronous and happens on the event loop thread, so there's no actual concurrency issue here. But it's worth noting that the factory creates a new V8 isolate synchronously in startRequest(), which blocks the event loop. For workers with large codebases, this could cause noticeable latency on the first request after an abort.

Also: if workerFactory() throws (e.g., compilation error in the worker script), isolateAbortRequested remains true (since the assignment at line 2105 is after the factory call). This means subsequent requests will keep trying to recreate and failing. Consider catching the exception and logging it, or at least resetting the flag regardless.

[LOW] Previously flagged by anonrig: this function also needs to be added to the `types/` folder (with `just generate-types`). kentonv confirmed the `never` return type is correct (since the function does not return).

[LOW] The abort-all mechanism looks correct: rejecting the fulfiller causes all `onLimitsExceeded()` branches to fire, which triggers `abortWhen()` -> `abort()` on each IoContext. The subsequent `resetAbortAllPromise()` creates a fresh pair for the next generation.

One subtlety: between rejecting the old fulfiller and calling `resetAbortAllPromise()`, if any code path called `onLimitsExceeded()` (e.g., a new IoContext being constructed), it would get a branch of the already-rejected promise and immediately abort. This is actually the desired behavior (we want requests on the old isolate to fail), but worth noting in a comment.